impr(streak hour offset): support 30 min offsets (@large-r0dent) #7269
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
large-r0dent
changed the title
2 tasks
Member
|
hi @large-r0dent , thank you for your contribution. Can you remove the useless comments? For example this comment is just repeating the actual code const inputValue = parseFloat( // parse float to support fractional stuffThis comment mentiones an issue but not which issue and does not help understand the code. // 0.0 or 0.5 (assuming we only wanna allow 30 min offsets like in original issue)
if (value < -11 || value > 12 || (value % 1 !== 0 && value % 1 !== 0.5)) {What about const isInvalidHour = value < -11 || value > 12;
const isInvalidMinute = value % 1 !== 0 && value % 1 !== 0.5; //we only support no or thirty minute offsets
if(isInvalidHour || isInvalideMinute) {This comments are just repeating the name of the constants Please check the other comments as well. |
Member
|
Please add |
fehmer
requested changes
Dec 19, 2025
Member
fehmer
left a comment
fehmer
left a comment
There was a problem hiding this comment.
Hi @large-r0dent,
please take a look at my comments. Have you tested your changes with a local development environment?
Leonabcd123
reviewed
Dec 19, 2025
github-actions
bot
added
the
waiting for update
Contributor
Author
|
@fehmer @Leonabcd123 sorry, new to oss contribution. will fix + poc test |
github-actions
bot
removed
the
waiting for update
github-actions
bot
added
the
waiting for review
Contributor
|
Continuous integration check(s) failed. Please review the failing check's logs and make the necessary changes. |
github-actions
bot
added
waiting for update
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds support for 30-minute increments in streak hour offset configuration. Previously, users could only set offsets in whole hour increments; now they can use values like 5.5 to represent 5 hours and 30 minutes.
Key Changes:
- Modified parsing logic to use
parseFloatinstead ofparseIntto handle fractional values - Updated date manipulation to calculate hours and minutes separately from the fractional input
- Enhanced validation to allow values ending in .5 for 30-minute increments
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
frontend/src/ts/modals/streak-hour-offset.ts |
Updated parsing logic to support floats, refactored date manipulation to handle fractional hours, and updated validation and error messages |
frontend/src/html/popups.html |
Added step attribute to allow 0.5 increments in the number input |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
github-actions
bot
removed
the
waiting for update
Leonabcd123
reviewed
Dec 20, 2025
fehmer
reviewed
Dec 20, 2025
Miodec
added
the
waiting for update
github-actions
bot
removed
the
waiting for update
Miodec
changed the title
3 tasks
Miodec
pushed a commit
that referenced
this pull request
Jan 14, 2026
…nya-Keyal) (#7363) ### Description The current streak hour offset validation fails for values like `-5.5` because `-5.5 % 1` evaluates to `-0.5`, causing the fractional check to fail. This PR simplifies the validation logic by multiplying the value by 2 and checking whether the result is an integer. This correctly allows only whole-hour and half-hour (`.5`) offsets while handling negative values properly. <img width="1667" height="1320" alt="Screenshot 2026-01-14 203300" src="https://github.com/user-attachments/assets/3c5ccef5-a173-4df7-9ff2-037d4417ae10" /> ### Checks - [x] Check if any open issues are related to this PR; if so, be sure to tag them below. - [x] Make sure the PR title follows the Conventional Commits standard. (https://www.conventionalcommits.org for more info) - [x] Make sure to include your GitHub username prefixed with @ inside parentheses at the end of the PR title. <!-- label(optional scope): pull request title (@your_github_username) --> <!-- I know I know they seem boring but please do them, they help us and you will find out it also helps you.--> Closes #7204, related PRs #7269, #7362 <!-- the issue(s) your PR resolves if any (delete if that is not the case) --> <!-- please also reference any issues and or PRs related to your pull request --> <!-- Also remove it if you are not following any issues. --> <!-- pro tip: you can mention an issue, PR, or discussion on GitHub by referencing its hash number e.g: [#1234](#1234) --> <!-- pro tip: you can press . (dot or period) in the code tab of any GitHub repo to get access to GitHub's VS Code web editor Enjoy! :) -->
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
add 30 minute streak offsets as described in #7204
i looked at the backend and i'm pretty sure that no changes are needed there since there's nothing that works with it that wouldn't work with floats.